-
Notifications
You must be signed in to change notification settings - Fork 432
feat: [cli] Pass optional args to get matches #2787
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also do you mind also add a change file like this one?
Added a new `global` boolean flag to the `CliArg` struct to support global CLI arguments. This flag allows arguments like `--verbose` to be marked as global and automatically propagated to all subcommands, enabling consistent availability throughout the CLI. |
Package Changes Through 9763dc2There are 2 changes which include cli with minor, cli-js with minor Planned Package VersionsThe following package releases are the planned based on the context of changes in this pull request.
Add another change file through the GitHub UI by following this link. Read about change files or the docs at github.com/jbolda/covector |
Can maybe scrounge together a docs update too if you think it would help! |
About the clone trait thing, I don't mind we add derive |
pub fn get_matches( | ||
cli: &Config, | ||
package_info: &PackageInfo, | ||
args: Option<Vec<String>>, | ||
) -> crate::Result<Matches> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Legend-Master Curious and slightly bored here, hope you don't mind me asking. You called out the changes to Cli.matches
as a breaking change, how is this also not considered a breaking change? Both Cli.matches
and parser::get_matches
are public, and neither are referenced anywhere in the usage guide.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies, did not notice that the docs do mention Cli.matches
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
parser
is a private module, so parser::get_matches
is not actually public, yeah I know it's kinda confusing 😂
I tried that in my testing but Rust then complained about
|
You'll need to also add it to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
This makes it possible for people to use the CLI plugin in tandem with the single-instance plugin:
Note it's still a little awkward to actually use. If people want to send the matches to the client, they need to encode / decode them themselves as
emit_to
expects the data to be clone-able.eg Here's what I have to do on the Rust side:
and on the client side:
Closes #2785